Update useModalOverlay types #5116
Conversation
Add extra types from useOverlay props
|
Sorry for the close/re-open spam - was trying to shake the CLABot out of its slumber 😆 |
|
I've actually had to do something similar when implementing SubMenus for |
|
Right on, thanks. In case it's useful, the specific use case here came up when with our design system's Dialog component. When a Dialog contained a legacy Select component, attempting to select an entry from the dropdown would close the dialog.
|
| */ | ||
| isKeyboardDismissDisabled?: boolean | ||
| } | ||
| export interface AriaModalOverlayProps extends AriaOverlayProps {} |
There was a problem hiding this comment.
We discussed adding shouldCloseOnInteractOutside to usePopover for submenus today, leaning towards being fine adding it there and for useModalOverlay as well as you've done here. I'd personally would prefer just adding shouldCloseOnInteractOutside only here instead of all the overlay props until we get use cases for the rest of those props but happy for the rest of the team to weigh in.
Would you mind adding a basic test for this? You can probably add a new test file for useModalOverlay or for the RAC Modal since it extends from AriaModalOverlayProps. https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/overlays/test/useModal.test.js or any of the tests in https://github.com/adobe/react-spectrum/tree/main/packages/react-aria-components/test could serve as a guide
There was a problem hiding this comment.
Thanks for the review and sorry for the delay, we had an onsite at work which naturally took all my attention last week.
Good to hear - I'll look to hack together a test this week.
devongovett
force-pushed
the
main
branch
2 times, most recently
from
8cb1a5d to
3013156
Compare
devongovett
force-pushed
the
main
branch
2 times, most recently
from
a79adcf to
3013156
Compare
|
This is now supported via #5986. |
3 participants
Extended the type of the
useModalOverlay()props fromuseOverlay()'s props.I want to be able to pass
shouldCloseOnInteractOutsidetouseModalOverlayso that I can prevent closing in certain cases.The values are already being passed through to
useOverlay()so I went ahead and simply extendedAriaOverlayPropsinstead of adding more and more repeating variables. I'm not sure if there's a particular reason that approach wasn't taken already, so feel free to recommend the alternative.It seems like any docs for this should be updated automatically but lemme know if I'm missing anything here.
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: